v0.7.40: library, mcp hardening, pi code reviewer, gmail fix#5761
Merged
Conversation
* feat(library): add Best Relay.app Alternatives in 2026 post * improvement(library): clarify Sim free tier has usage limits in comparison table
…ghs DoS) (#5756) * fix(security): bound YAML alias expansion in file-parser to prevent DoS The YAML parser called yaml.load() then JSON.stringify() on the result. YAML aliases (*anchor) resolve to shared references, so yaml.load cheaply builds a compact DAG, but JSON.stringify expands that DAG into a full tree, duplicating every shared node. A crafted sub-1KB .yaml/.yml document could therefore expand to hundreds of MB / GB during serialization, pinning CPU and OOM-killing the shared parse/ingestion worker (CWE-776). Add a bounded, iterative traversal that runs before JSON.stringify and aborts once expanded node count, estimated serialized size, or nesting depth exceeds safe caps. This detects the amplification against the compact DAG before any allocation, and also terminates cyclic anchor structures. Replaces the unbounded recursive getYamlDepth (which spread large arrays into Math.max(...array), risking a stack overflow) with the same bounded walk. * harden YAML guard: charge keys + escaping, bound stack, fail closed Addresses review findings on the alias-expansion guard: - Charge object keys, not just values. JSON.stringify re-emits every key on each alias expansion, so an aliased object with a long key amplified without being counted. Keys are now charged against the size cap. - Compute the exact JSON-escaped string length (quotes, backslashes, control chars) instead of a flat multiplier. Plain text is charged its true 1:1 size (no false rejection of large legitimate documents) while escape-heavy strings are charged their real, larger cost (no cap bypass). - Count each node as it is enqueued and only push container nodes onto the traversal stack. A pathologically wide fan-out now trips a cap during the enqueue loop instead of first materializing millions of stack entries and exhausting memory inside the guard itself. - Fail closed in POST /api/files/parse: a YamlComplexityError rejection is now re-thrown (mirroring isPayloadSizeLimitError) rather than silently falling back to storing the crafted document as raw text. * yaml guard: charge lone surrogates at their escaped length serializedStringLength treated surrogate code units as cost 1, but well-formed JSON.stringify escapes a lone surrogate to \uXXXX (six units). Charge lone surrogates as 6 and valid high+low pairs as-is (two units), matching JSON.stringify exactly. Verified against JSON.stringify across plain text, escapes, control chars, lone high/low surrogates, and valid pairs.
* feat(library): add BYOK multi-model AI agent builder post * fix(library): scope BYOK FAQ to providers in the actual BYOK contract
…TP/2 (#5757) * improvement(mcp): negotiate HTTP/2 for MCP transport MCP servers are commonly behind HTTP/2 fronts (CDNs, cloud LBs), but the SSRF-pinned undici Agent is HTTP/1.1-only unless it opts into h2 via ALPN. Add an allowH2 option to createPinnedFetch (default false, so LLM-provider callers are unchanged) and centralize the MCP h2 decision in one createPinnedMcpFetch routed through the transport, OAuth probe, and SSRF-guarded fetch. Pinning is unaffected: the pinned lookup forces every connection to the resolved IP. * fix(mcp): correct auth-type handling, OAuth-pending UX, and safe error logging - Classify a non-OAuth UnauthorizedError as an auth failure instead of an OAuth redirect, so static-bearer servers that merely advertise OAuth stop being diverted into the OAuth flow (fixes the class of server that couldn't connect). - Reset a server to disconnected when it is switched to OAuth, so it can't falsely read as connected before completing its auth flow. - Surface OAuth-pending state as 'OAuth authorization required' in the server list and refresh action instead of a generic 'Not Connected'. - Return an actionable 422 for OAuth servers that don't support dynamic client registration. - Redact error messages/cause/session-ids from MCP transport/connect logs via a shared getMcpSafeErrorDiagnostics helper. * fix(mcp): reset connection on any auth-type flip, scope h2 to live transport * fix(mcp): close pinned h2 Agent on disconnect and revoke OAuth tokens on auth-type change * fix(mcp): release pinned Agent on failed connect and point DCR error at client-id/secret setup * fix(mcp): make pinned Agent teardown idempotent to avoid double-destroy on cancel/failed connect
…5760) * feat(mcp): reuse warm connections for tool execution and discovery * fix(mcp): make connection pool concurrency-safe and auth-scoped * fix(mcp): decouple pool validity from updatedAt, widen dead-connection detection, guard ping race * fix(mcp): retire pooled connections on auth failure and server delete * improvement(mcp): defer bulk-discovery env resolution to the pool miss path * fix(mcp): retire in-flight creates evicted mid-connect via server generation * fix(mcp): use evicted-mid-create connections one-shot and decouple pool eviction from cache clear * improvement(mcp): dedupe create thunks into buildClient, harden dispose against liveness race * fix(mcp): close acquire-gap races (re-resolve after stale ping, skip closing entries) * fix(mcp): retry auth failures in executeTool; simplify acquire re-check and clear generations on dispose * fix(mcp): let bulk discovery reconnect a lost notification connection with current config * fix(mcp): recover rotated credentials on discovery via shared fetchServerTools auth-retry * chore(mcp): add debug logging for pool hit/miss/eviction observability
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview MCP: Tool execution and discovery reuse warm connections per Workflows: Loading latest execution state only passes non-null Content: Two new library articles (Relay.app alternatives, BYOK multi-model builder). Reviewed by Cursor Bugbot for commit bd636d4. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bd636d4. Configure here.
| */ | ||
| export function isYamlComplexityError(error: unknown): error is YamlComplexityError { | ||
| return error instanceof YamlComplexityError | ||
| } |
There was a problem hiding this comment.
YAML fail-closed check can miss
High Severity
isYamlComplexityError relies only on instanceof, but the complexity error is thrown from the YAML parser loaded via require() in file-parsers/index.ts while the parse route checks it through a static import. On Next.js 16 (Turbopack by default), that can yield two class identities, so the guard fails, the handler falls back to raw-text parsing, and a rejected alias bomb is returned as success: true instead of fail-closed.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit bd636d4. Configure here.
Contributor
Greptile SummaryThis release bundles a YAML alias-expansion DoS fix, a warm MCP connection pool, OAuth UX improvements, and a chat execution-state loader bug fix. The changes are well-scoped and thoroughly tested.
Confidence Score: 4/5Safe to merge; no regressions in core data paths and the security fix is correctly implemented end-to-end. The YAML fix is well-hardened and properly propagated through the fallback path. The connection pool handles the key concurrency races correctly. Two things keep the score from being higher: the DCR error detection in the OAuth route relies on a brittle substring match that would silently degrade if the SDK message changes, and the serverGenerations map in the pool accumulates entries for every server ever evicted without pruning, which could matter for long-running processes with high server churn. apps/sim/lib/mcp/connection-pool.ts (serverGenerations growth) and apps/sim/app/api/mcp/oauth/start/route.ts (string-match DCR detection) deserve a second look. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Caller as Tool Executor / Discovery
participant Service as McpService
participant Pool as McpConnectionPool
participant Client as McpClient
participant Server as MCP Server
Caller->>Service: executeTool() / discoverServerTools()
Service->>Pool: acquire(key, serverId, create)
alt Pool hit
Pool->>Pool: tryReuse(entry): ping if stale
Pool-->>Service: ConnectionLease (existing client)
else Pool miss
Pool->>Service: invoke create()
Service->>Service: resolveConfigEnvVars + SSRF pin
Service->>Client: new McpClient(pinnedFetch + H2)
Client->>Server: connect()
Server-->>Client: initialized
Client-->>Service: connected McpClient
Service-->>Pool: entry registered
Pool-->>Service: ConnectionLease
end
Service->>Client: listTools() / callTool()
Client->>Server: tools/list or tools/call
Server-->>Client: result
Client-->>Service: result
alt Success
Service->>Pool: "lease.release(poison=false)"
Pool->>Pool: keep connection warm
else Dead connection (400/401/404/reset)
Service->>Pool: "lease.release(poison=true)"
Pool->>Pool: "retire entry, closeIfIdle when borrowers=0"
end
Service-->>Caller: result / error
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Caller as Tool Executor / Discovery
participant Service as McpService
participant Pool as McpConnectionPool
participant Client as McpClient
participant Server as MCP Server
Caller->>Service: executeTool() / discoverServerTools()
Service->>Pool: acquire(key, serverId, create)
alt Pool hit
Pool->>Pool: tryReuse(entry): ping if stale
Pool-->>Service: ConnectionLease (existing client)
else Pool miss
Pool->>Service: invoke create()
Service->>Service: resolveConfigEnvVars + SSRF pin
Service->>Client: new McpClient(pinnedFetch + H2)
Client->>Server: connect()
Server-->>Client: initialized
Client-->>Service: connected McpClient
Service-->>Pool: entry registered
Pool-->>Service: ConnectionLease
end
Service->>Client: listTools() / callTool()
Client->>Server: tools/list or tools/call
Server-->>Client: result
Client-->>Service: result
alt Success
Service->>Pool: "lease.release(poison=false)"
Pool->>Pool: keep connection warm
else Dead connection (400/401/404/reset)
Service->>Pool: "lease.release(poison=true)"
Pool->>Pool: "retire entry, closeIfIdle when borrowers=0"
end
Service-->>Caller: result / error
Reviews (1): Last reviewed commit: "feat(mcp): reuse warm connections for to..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
#5762) Response-side Zod .catch() tolerance on the three strict-enum-over-free-text MCP columns (transport/authType/connectionStatus) so one legacy row can't fail the whole server-list validation; fork copy normalizes transport; create/upsert path resets connection status on any auth-type flip (mirrors update path); bulk discovery drops the positive tool cache on OAuth-pending. Request bodies stay strict.
… can't strand the connect (#5767) * fix(mcp): deliver OAuth callback result over BroadcastChannel so COOP can't strand the connect An MCP provider whose authorization page sets `Cross-Origin-Opener-Policy: same-origin` (e.g. Gauge) severs `window.opener` when the popup navigates through it, so the callback's `window.opener.postMessage` was silently lost and the row hung on "Connecting…" forever — even when the callback succeeded. - Signal completion over a same-origin `BroadcastChannel` instead, which is origin-scoped and immune to opener severance (the MDN/Chrome-recommended COOP workaround). Scope the message by workspaceId so other open workspaces ignore it. - Log every OAuth callback failure with its reason + serverId. The early-return gates previously returned a silent `ok:false` popup close, so a failed authorization was undiagnosable from the server logs. * fix(mcp): react to the OAuth broadcast only in the tab that opened the popup A BroadcastChannel reaches every same-origin tab, so the previous workspace-id scoping (which the success path carried but failure paths omitted) still let unrelated tabs clear state, refetch, and show spurious toasts. Gate every reaction on whether this tab actually has an in-flight popup for the result's server (`popupIntervalsRef`) — strictly more precise than workspace scoping and correct for both cross-workspace and same-workspace-second-tab cases. Removes the now-redundant workspaceId from the callback message. * fix(mcp): decouple OAuth result correlation from popup.closed Cursor flagged two defects in the previous gate, both rooted in keying the BroadcastChannel filter on `popupIntervalsRef` (the popup.closed poll map): - A genuine completion was dropped whenever the poll had already removed the server's entry — and COOP can make `popup.closed` misreport, which is exactly the case this PR targets, so the fix could silently fail to apply. - A result without a serverId fell back to "any in-flight popup", waking unrelated same-origin tabs with spurious toasts/refetches. Introduce `pendingFlowsRef` (serverId -> safety timeout) as the correlation source of truth: cleared only on completion or a 10-min timeout (matching the server OAuth start TTL), never by popup.closed. The popup.closed poll now only clears the spinner (best-effort abandon UX) and never touches correlation, so a real completion is always processed. Results without a serverId are ignored; the initiating tab's safety timeout clears its own "Connecting…". * fix(mcp): correlate the OAuth result on the state nonce, not serverId Cursor flagged that a failure which can't resolve a serverId (notably invalid_state) broadcasts ok:false with no serverId, so the serverId-keyed gate ignored it and the initiating tab sat on "Connecting…" until the safety timeout with no error feedback. Correlate on the OAuth `state` instead — the per-flow nonce the callback echoes on every result, success or failure. The client parses it from the authorization URL and keys the in-flight map by it; the callback includes it on every response via a `respond` helper. This is the canonical popup-OAuth correlation: it reaches the initiating tab even when no serverId exists, and — because each flow has a unique state — it also fixes the same-user-same-server-in-two-tabs edge a serverId key left open. Results with no parseable state (a malformed callback) are still ignored; that flow clears via its timeout. * fix(mcp): drop a server's prior in-flight OAuth flow when it is retried Each start mints a new `state`, so an abandoned attempt's safety timeout was keyed under a different state than its retry and never cleared. In the contrived case where both stayed pending ~10 min, the stale timer would clear the newer flow's spinner. Sweep any prior in-flight flow for the same serverId on a new start (replacing the can't-happen same-state check). Result delivery was already correct; this makes the state machine airtight.
* Simplify TikTok to draft-only Content Posting Remove Direct Post stubs and legacy Share Kit webhook triggers that Sim does not ship, and fix draft upload response handling so real TikTok errors are not misreported as size-limit failures. Co-authored-by: Cursor <[email protected]> * fix TikTok cleanup lint and Greptile review findings Reject legacy mode:direct publish requests instead of silently drafting, keep deprecated Share Kit triggers registered for saved workflows, and apply biome format/import fixes. Co-authored-by: Cursor <[email protected]> * Finish TikTok greenfield draft-only surface Remove Query Creator Info and video.publish, drop Share Kit trigger stubs, rename publish-video to upload-video-draft, and strip leftover Direct Post mode from the contract. Co-authored-by: Cursor <[email protected]> * Hide TikTok from the toolbar until app review is approved. Co-authored-by: Cursor <[email protected]> --------- Co-authored-by: Bill Leoutsakos <[email protected]> Co-authored-by: Cursor <[email protected]>
…te to the agent (#5656) Implement the sim-side share_file server tool (resolves VFS path to file, keeps the existing org-policy + permission + audit checks, delegates to upsertFileShare). Register it in the tool router and generated catalog. Stamp an ambient 'shared'/'shareAuthType' flag onto file metadata via one batched share lookup, mirroring how workflows expose 'isDeployed'. Validate the EFFECTIVE auth type when re-enabling a share: upsertFileShare preserves an existing share's authType when none is passed, so validate the stored mode (not 'public') or a re-share could reactivate a now-disallowed password/email/sso share. Same fix applied to the share PUT route.
github-actions
Bot
added
the
requires-mothership-merge
|
…clobber (#5772) The MCP OAuth callback fails with invalid_state because the authorization state is cleared/missing by the time the user authorizes — but clearState was silent, so the clobber never surfaced in logs. Log every state save and clear with a caller context so the exact source is visible on the next repro.
…rand over first/last rows (#5774)
…; document DCR match (#5777)
…emplate title (#5775) * improvement(blocks): name the integration in every suggested-action template title Home-screen "Suggested actions" rows render only an icon + the block template title, so titles that did not name their integration (e.g. "Refund pattern monitor") were unidentifiable. Rewrite the 234 titles that omitted their integration so each names it naturally, matching each block's existing "Brand + phrase" style. Titles-only change; the catalog surfaces that already show the integration as page context are unaffected. * fix(blocks): name PagerDuty-triggered incident template after its icon, not owner The Confluence-owned incident template uses a PagerDutyIcon, is triggered by PagerDuty, and is cross-listed to notion/pagerduty/datadog/slack, so prefixing the owner name misidentified it on those surfaces and fought its own icon. Name it after the icon + trigger integration instead. * fix(blocks): align suggested-action icons with titles and de-dupe Firecrawl names For rows where the owner-prefixed title named a different integration than the row's icon (github/Notion, stripe/Sheets, calendar/Twilio, gmail/Lemlist, slack/Linear, linear/Slack), swap the icon to the owner integration the title already names so the icon-only home row is coherent; drop the now-unused icon imports. Differentiate two Firecrawl titles that collided with existing near-duplicate templates. * fix(blocks): de-duplicate suggested-action titles the brand prefix collided Renamed a handful of titles whose brand prefix made them near-identical to a sibling template: google_translate (Intercom replier / ticket auto-reply), sentry (on-call triage agent), google_drive (personal notes assistant), sqs (alert enricher), typeform (survey summarizer). Swept every renamed block for the same near-duplicate class. * fix(blocks): align Greptile Slack Q&A bot icon with its title The renamed title leads with Greptile but the row kept icon: SlackIcon, conflicting on the icon-only home surface. Swap to GreptileIcon (matching the block's sibling templates) and drop the orphaned SlackIcon import.
* feat(pi): add Cloud Code Review mode and rename Cloud to Cloud PR Introduce a third Pi mode that reviews an existing GitHub PR in an E2B sandbox and posts a structured review with optional inline comments. Keep the stored cloud id for backward compatibility, extend github_create_pr_review for inline comments, and harden review submission against stale SHAs and invalid comment payloads. Co-authored-by: Cursor <[email protected]> * chore(pi): cleanup code * address comments * address mor * address comments * update --------- Co-authored-by: Bill Leoutsakos <[email protected]> Co-authored-by: Cursor <[email protected]> Co-authored-by: Vikhyath Mondreti <[email protected]>
…ds (#5783) * fix(library): show full unambiguous dates on featured and related cards * fix(library): format content dates in UTC to avoid off-by-one day
#5781) * improvement(ux): submit on Enter in chip modals and advance table rows * fix(emcn): stop Enter double-submit and close registration gap - stopPropagation on field Enter-submit so a parent onKeyDown can't re-fire the primary (double OAuth connect, etc.) - register primary via useLayoutEffect so it's set before paint (no null-Enter window) - drop now-redundant parent Enter handlers in connect-oauth/create-workspace/rename-document modals * fix(table): suppress auto-opened tag dropdown when Enter advances cells Focusing an empty cell auto-opens the tag dropdown; without closing it a follow-up Enter inserts a tag instead of navigating down the column. * fix(table): clean up cell refs on unmount and guard Enter advance - delete inputRefs/overlayRefs entries when a cell detaches so deleting a row can't leave stale position-keyed entries - guard Enter advance with isConnected so a stale ref can never steal focus into a detached node
…nsport (#5782) * diag(mcp): comprehensive HTTP logging for OAuth + streamable-HTTP transport Temporary diagnostic to root-cause the Gauge MCP 'initialize' hang. Wraps both the OAuth guarded fetch and the transport pinned fetch to log every request/response with timing: method, url, status, content-type, mcp-session-id, safe headers, and — for the transport phase only — the response body streamed chunk-by-chunk. So one authorize reveals exactly what Gauge returns for initialize (SSE that stalls vs never-sent result vs fast JSON) and where it stalls. Enabled by default; MCP_HTTP_DIAGNOSTICS=false to silence; disabled under test. Never logs request bodies or the OAuth token-response body (carry tokens); every logged value passes through sanitizeForLogging. Revert once root-caused. * diag(mcp): scope body logging to initialize + redact URLs + wrap unpinned Review fixes (Greptile/Cursor): - Only stream-log the response body whose REQUEST is an MCP initialize JSON-RPC call. The transport fetch also carries in-transport OAuth refresh and every tools/call result, so gating on phase alone leaked token responses and tool output (PII/file contents). initialize gating excludes both. - Redact URL query strings (?code=/?token=/?api_key=) — log origin+path only. - Wrap the transport fetch whether pinned or not (SDK default is globalThis.fetch, so passing it is behavior-neutral) so unpinned/allowlisted servers are covered too. * diag(mcp): sanitize initialize preview + log at warn for visibility Review fixes (Cursor): - Run the initialize body preview through sanitizeForLogging (defense-in-depth against a server stuffing token-like values into serverInfo/capabilities). - Log diagnostics at warn, not info, so they surface even if an environment's LOG_LEVEL drops info (staging runs INFO, but this removes the dependency). * diag(mcp): reuse sanitizeUrlForLog + cancel log reader on abort Review fixes (Cursor): - Replace the local safeUrl helper with the shared sanitizeUrlForLog so URL redaction/truncation stays consistent. - The detached initialize-body log reader now observes init.signal and cancels its tee-branch reader on abort, so it can't keep the response stream / connection alive after the SDK's initialize timeout gives up (the hang we're tracing). * diag(mcp): length-gate initialize check to skip parsing large tool payloads isInitializeRequest now rejects bodies over 4096 chars before any JSON.parse. The initialize message is a small fixed structure, so this keeps large tools/call payloads (potentially multi-MB) off the parse hot path while still detecting initialize.
…5785) * feat(blocks): surface deprecated block and model warnings on canvas * improvement(blocks): simplify deprecation derivation, drop unused getModelReplacement * fix(blocks): make deprecation badge keyboard-accessible
* zip checkpoint * fix(zip-uploads): harden extract path from review findings - Replace the whole-buffer central-directory signature scan with an EOCD-anchored walk (shared with zip-guard) so zips containing STORED nested archives are no longer falsely rejected, and report the accurate cap (new 'central_dir_too_large' reason) instead of 'Maximum is 1000' - Make extraction all-or-nothing: a discarding validation pass inflates every entry against the caps before anything is uploaded, so lying headers or corrupt entries can't leave partial trees (corrupt DEFLATE streams now surface as ArchiveError instead of raw zlib errors) - Single-source ArchiveError messages; callers surface err.message and map only status/reason (removes the three divergent message maps) - Guard save/import against archives (saving stranded the contents), extend the workspace ownership check to all three operations, dedupe fileNames, and refuse re-extracting into a non-empty folder instead of duplicating the tree with ' (1)' copies - Harden the extraction folder name (dot segments, control chars, separators) and compute the destination before extracting - Sniff small '.zip'-named uploads so mislabeled text files stay readable instead of dead-ending between read and extract - Scope zip acceptance to the mothership flow only (attachment list, accept attribute, upload/presigned gates) so execution/workspace/chat surfaces keep rejecting zips up front - Don't count skipped noise entries toward the 1000-file cap; restore per-entry zip-slip forensics via skippedUnsafePaths logging - Teach materialize_file(fileNames: [...]) in the extract guidance to match the declared schema * fix(zip-uploads): address review follow-ups on the extract path - Roll back already-uploaded files when an upload fails mid-extraction (storage/DB error, quota crossed), so callers and retries never observe a partial tree - Fold reserved system folder names (.changelogs, .plans) into the 'archive' fallback so extraction can't write into alias-backing namespaces or bypass the already-extracted lookup that hides them - Align the archive byte-sniff budget with the read path's inline text cap (5MB), so any mislabeled '.zip' small enough to be read inline is sniffed and read instead of dead-ending * fix(zip-uploads): detect prior nested-only extractions in the re-extract guard The already-extracted check only looked for files directly inside the archive root folder, so a zip whose entries are all nested (src/index.ts) left only subfolders there and a second extract slipped past the guard, duplicating the tree with ' (1)' suffixes. Extraction roots its whole tree at that folder, so a prior run always leaves a direct file OR a direct subfolder — check both.
…ng (#5789) The SSRF-guarded fetch used for MCP OAuth (discovery, DCR, token exchange/ refresh, RFC 7009 revocation) created a fresh pinned undici Agent per request and never tore it down, and returned the live response so the SDK's lazy body read happened outside any deadline. The MCP SDK sets no timeout on OAuth legs, so a stalled body read or a leaked keep-alive socket could leave the flow — and the browser waiting on it — pending indefinitely ("Connecting… forever"). - Buffer the (always-small) OAuth JSON body inside the guard, under the composed deadline/caller AbortSignal, then return a detached in-memory Response. undici's bodyTimeout only measures idle gaps between chunks and cannot bound a slow-drip body; the AbortSignal is the only true wall-clock deadline over the body read. - Destroy (not close) the per-request pinned Agent on every path so a one-shot leg can't strand its keep-alive socket, and teardown itself can't hang. - Fix the same per-request Agent leak in the auth-type probe. - Returning a normalized global Response also surfaces provider token-endpoint errors cleanly instead of the SDK's opaque parse failure.
waleedlatif1
changed the title
…d red) (#5793) * improvement(blocks): two-tier sunset schema (legacy amber / deprecated red) * fix(blocks): require replacedBy on legacy sunset blocks in registry check * improvement(blocks): reword legacy badge tooltip
* feat(managed-agents): add Claude Managed Agents workflow block
* improvement(managed-agents): complete session inputs/outputs; trim block templates
- add memory instructions + file mount_path inputs; bound metadata (16 pairs)
- surface cumulative token usage (inputTokens/outputTokens) as outputs
- validate the full session-create schema against live docs
- remove BlockMeta templates
* improvement(managed-agents): select a Claude Platform credential instead of BYOK
- register Claude Platform as a token-paste service-account credential (descriptor + validator)
- add no-OAuth OAUTH_PROVIDERS entry; generalize the shared credential picker with a 'service-account' kind
- block: oauth-input credential picker + dependsOn dropdowns; list route resolves the key server-side (audit-logged)
- run via directExecution with the executor-injected key; drop the internal run route
- remove the interim claude-platform BYOK provider
* fix(managed-agents): harden reconnect loop; fix credential-picker regression
- drive completion off terminal events + authoritative session status (drop the fragile busy-clock)
- drain full event history so a long session's tail is never cut off
- skip idless events in catch-up; require an id before replying to custom_tool_use
- gate the shared credential-selector service-account lookup on credentialKind and use the non-throwing helper (was crashing multi-service OAuth pickers)
- audit-log the list route's credential access; refresh stale tool docs
* fix(managed-agents): address review round 2
- don't complete on idle status while a requires_action is still outstanding
- vaults: combobox → dropdown so multiSelect actually attaches multiple vaults
- auto-select a freshly-pasted service-account credential (onCreated through the connect modal)
* fix(managed-agents): propagate cancellation, fix reconnect ordering, classify advanced fields
- Thread the executor abort signal into directExecution (additive ToolConfig
change) so a cancelled workflow stops the session immediately; best-effort
user.interrupt releases the Anthropic session past cancel/wall-clock cap
- Recompute the requires_action pending state from the chronological history
so an older agent.message recovered on catch-up can't clear a newer pause
- Retry a custom-tool error reply that failed to send instead of stranding the
session (mark the event seen only once handled)
- Mark optional fields (vaults, memory, files, metadata) mode: advanced
- Title the session with the workflow id for Claude-console traceability
* fix(managed-agents): stop leaked sessions on all give-up paths; harden event ordering and normalizers
- Interrupt on sendUserMessage failure and on the reconnect-cap exit, matching
the abort/wall-clock paths, so no give-up path leaves a session running
- Order the events list by processed_at (list page order isn't guaranteed
chronological) so catch-up accumulates text and reads the latest lifecycle
event correctly regardless of API sort
- Don't reset reconnect backoff on a failing custom-tool reply (stays unseen
for retry) — prevents a no-delay reconnect storm
- Treat only end_turn as a complete status_idle event; an unspecified idle
defers to the sawActivity-gated status check (no empty completion pre-turn)
- normalizeFiles parses a JSON-stringified table instead of dropping it;
normalizeStringList returns [] on malformed JSON; metadata keeps scalar values
- Declare the injected accessToken as a hidden param for convention parity
* fix(managed-agents): interrupt the session on a mid-run stream/API failure
The non-abort error catch path returned without stopping the session, so a
mid-run network/API failure could leave the Anthropic session running against
the workspace key. Interrupt on that path too — completing the invariant that
every give-up exit (abort, cap, send failure, reconnect cap, stream error)
stops the session. Still fails fast; no retry added.
* fix(managed-agents): skip idless stream previews; resolve service-account provider icons
- Skip idless events in the live SSE handler (mirroring catch-up). event_start/
event_delta previews carry no id, are never deduped, and final text always
arrives as a persisted id-bearing agent.message — so appending previews could
double the block's content output
- Map serviceAccountProviderId to its base provider in PROVIDER_ID_TO_BASE_PROVIDER
so parseProvider resolves 'claude-platform-service-account' to 'claude-platform'
(a two-segment base the hyphen split can't recover), fixing the credential-row
icon falling back to the generic external-link glyph
* fix(managed-agents): keep idless terminals and preserve live pending state
Refine the round-6 idless-event fix, which was too broad:
- Process idless events again (revert the blanket stream skip) so an idless
session.status_idle(end_turn)/session.error delivered only on the live stream
still registers as terminal instead of reconnecting to a timeout. Only the
agent.message TEXT append is now id-gated, so preview text still can't double
- When catch-up history has no lifecycle event, restore the live-observed
requires_action state instead of trusting a stale older agent.message that
cleared it — prevents a false completion with partial output while a tool
result is still pending
* fix(managed-agents): route memory via metadata on self-hosted environments
Live API testing revealed self-hosted environments reject the `resources`
array with a 400 ("resources are not supported with self-hosted
environments"), so the prior universal-resources[] payload would have failed
any self-hosted session that attached a memory store or files.
Restore env-type-aware routing: resolve the environment's config.type via a new
getEnvironmentType() before session create, and for self_hosted send the memory
store through metadata.memory_store_ids/memory_access (the worker consumes it)
and drop file attachments. Cloud environments keep resources[]. Verified end-to-
end against the live Managed Agents API (cloud resources[] 200, self-hosted
metadata 200, self-hosted resources[] 400).
* feat(managed-agents): environment-type selector; hide cloud-only fields on self-hosted
Collapse what #5769 split into two blocks into one, natively:
- Add an Environment type selector (Cloud / Self-hosted) that filters the
environment list to the matching type and gates cloud-only fields
- Memory store, memory access/instructions, and files are cloud-only (self-
hosted rejects the resources[] attach — verified live, 400) and are now hidden
on self-hosted instead of silently dropped. A self-hosted worker that uses a
memory store reads its id from a Metadata key the author sets explicitly
- Expose each environment's config.type on the list options so the picker can
filter by mode; pass the selected type as a routing hint (server still re-
resolves the authoritative type via getEnvironmentType)
* fix(managed-agents): track requires_action by processed_at, not history position
Persisted history can lag the live stream, so the last lifecycle event in the
history array may be OLDER than a requires_action the stream already observed.
Deriving the pending state from history position (findLastLifecycleEvent) could
then clear a newer pause and let an idle snapshot complete a still-waiting
session with partial text.
Track requires_action from the NEWEST lifecycle event by processed_at across
both the live stream and catch-up, so an older/lagging event can never override
a newer pause. Removes the pendingBeforeCatchup snapshot and history-position
recompute. New test covers lagging history holding only an older running event.
* fix(managed-agents): treat missing processed_at as oldest, not newest
A lifecycle event without processed_at mapped to +Infinity, poisoning the
high-water mark: once seen, no later timestamped event could update the pending
state (at >= Infinity always false), stranding a pause or clearing one wrongly.
Map missing/unparseable processed_at to -Infinity so an untimestamped lifecycle
event can never outrank a timestamped one in either direction — it neither
blocks later real events nor clears a timestamped requires_action. Persisted
lifecycle events always carry processed_at; this is purely defensive. New test
covers a stray untimestamped running not clearing a timestamped pause.
Reverts the temporary McpHttpDiag instrumentation from #5782 now that it has served its purpose: it confirmed the OAuth fetch fix (#5789) makes the flow reach and complete the token exchange, and it isolated a separate, pre-existing transport-initialize stall (Gauge returns 200 + session then no body for 30s, reproducible only from the staging egress — not from a fresh IP, and not with the SDK/h2/pinning/tee in isolation). - Delete apps/sim/lib/mcp/http-diagnostics.ts. - Restore client.ts transport fetch to `...(pinned ? { fetch: pinned.fetch } : {})`. - Restore oauth/auth.ts fetchFn to the plain SSRF-guarded default. - Drop the diagnostic-specific test assertion. Removes the body tee() from the transport hot path so the streamable-HTTP connection runs clean while the transport stall is investigated separately.
…2) (#5797) * fix(mcp): use HTTP/1.1 for the streamable-HTTP transport (drop allowH2) The MCP transport was the only place in the codebase opting undici into HTTP/2 (`allowH2: true`). undici's h2 support is experimental and has a documented cluster of stalls where response headers arrive but the body DATA frames never do, on POST bodies over reused/coalesced sessions (nodejs/undici #2311, #3433, #4143). Behind a shared egress IP fronted by a CDN, that is exactly what hung the streamable-HTTP `initialize` after OAuth: 200 + Mcp-Session-Id, then an empty body until the SDK's 30s timeout — reproducible only from the deployed egress, never from a fresh IP or in isolation. h2 buys the MCP transport nothing (one POST per JSON-RPC message plus a single long-lived SSE stream — no concurrency to multiplex), and both official MCP SDKs run the transport on HTTP/1.1: the TypeScript SDK's StreamableHTTPClientTransport calls global fetch (undici h1.1, no allowH2), and the Python SDK builds its httpx client with no http2=True. Dropping allowH2 aligns with them and steps off the h2 stall surface. CDN fronts serve h1.1 anyway; SSRF IP-pinning and Agent teardown are unchanged. * chore(mcp): trim verbose TSDoc and inline comments in pinned fetch + probe * chore(credentials): drop help text on the Claude Platform API key modal
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.